Skip to content

DOC: updated docstring for nanoseconds function per doc guidelines #21065

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
May 17, 2018

Conversation

chalmerlowe
Copy link
Contributor

  • closes #xxxx
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry
################################################################################
################### Docstring (pandas.Timedelta.nanoseconds) ###################
################################################################################

Return the number of nanoseconds (n), where 0 <= n < 1 microsecond.

Returns
-------
int : Number of nanoseconds

See Also
--------
Timedelta.components : Return all attributes with assigned values (i.e.
    days, seconds, microseconds, nanoseconds)

################################################################################
################################## Validation ##################################
################################################################################

Errors found:
	No extended summary found
	No examples section found

Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes - an Examples section would be a nice touch


Returns
-------
int : Number of nanoseconds
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep the type and description on separate lines. Make sure you indent the description and add a period at the end of it.

.components will return the shown components
See Also
--------
Timedelta.components : Return all attributes with assigned values (i.e.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put a period at the end of the description here. This isn't enforced and is inconsistent in the docs but should eventually be standardized


.components will return the shown components
See Also
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a reference to the other component attributes

@codecov
Copy link

codecov bot commented May 15, 2018

Codecov Report

Merging #21065 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #21065   +/-   ##
=======================================
  Coverage   91.83%   91.83%           
=======================================
  Files         153      153           
  Lines       49495    49495           
=======================================
  Hits        45454    45454           
  Misses       4041     4041
Flag Coverage Δ
#multiple 90.23% <ø> (ø) ⬆️
#single 41.88% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 501f041...4237391. Read the comment docs.

@chalmerlowe
Copy link
Contributor Author

Made the changes suggested...

################################################################################
################### Docstring (pandas.Timedelta.nanoseconds) ###################
################################################################################

Return the number of nanoseconds (n), where 0 <= n < 1 microsecond.

Returns
-------
int
    Number of nanoseconds.

See Also
--------
Timedelta.components : Return all attributes with assigned values (i.e.
    days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds).

Examples
--------
**Using string input**

>>> td = pd.Timedelta('1 days 2 min 3 us 42 ns')
>>> td.nanoseconds
42

**Using integer input**

>>> td = pd.Timedelta(42, unit='ns')
>>> td.nanoseconds
42

################################################################################
################################## Validation ##################################
################################################################################

Errors found:
	No extended summary found

Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - thanks!

@chalmerlowe
Copy link
Contributor Author

So... what is the resolution path if the travis-ci build fails?
not real familiar with Travis and thus not real sure where I should go from here.

The command "ci/lint.sh" exited with 1.
0.00s$ echo "checking imports"
checking imports

@WillAyd
Copy link
Member

WillAyd commented May 15, 2018

Typically things won't get merged if they have errors. You can look at the logs to see what happens - in this case your change is failing the LINT check:

pandas/_libs/tslibs/timedeltas.pyx:804:80: E501 line too long (84 > 79 characters)
pandas/_libs/tslibs/timedeltas.pyx:818:11: W291 trailing whitespace

You should get the same error locally if you run flake8 (see contributing guide for details)

@chalmerlowe
Copy link
Contributor Author

  • Removed the trailing whitespace.
  • Modified linebreaks to ensure that the text was within line length limits.
  • Ran the git diff upstream/master -u -- "*.py" | flake8 --diff command as required in the documentation guide.
################################################################################
################### Docstring (pandas.Timedelta.nanoseconds) ###################
################################################################################

Return the number of nanoseconds (n), where 0 <= n < 1 microsecond.

Returns
-------
int
    Number of nanoseconds.

See Also
--------
Timedelta.components : Return all attributes with assigned values (i.e.
    days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds).

Examples
--------
**Using string input**

>>> td = pd.Timedelta('1 days 2 min 3 us 42 ns')
>>> td.nanoseconds
42

**Using integer input**

>>> td = pd.Timedelta(42, unit='ns')
>>> td.nanoseconds
42

################################################################################
################################## Validation ##################################
################################################################################

Errors found:
	No extended summary found


Returns
-------
int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be on the same line with a colon? @TomAugspurger @jorisvandenbossche

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine as is. It's typically

name : type
    Explanation

but when the name is obvious (it'd just be nanoseconds here), it can be omitted.

@jreback jreback added Docs Datetime Datetime data dtype labels May 16, 2018
@jreback jreback added this to the 0.23.1 milestone May 17, 2018
@jreback jreback merged commit 9f40757 into pandas-dev:master May 17, 2018
@jreback
Copy link
Contributor

jreback commented May 17, 2018

thanks @chalmerlowe

@chalmerlowe
Copy link
Contributor Author

Thanks for closing. My First PR on pandas.

There was some back and forth comments on the formatting...

I followed the examples in the docstrings guide:

With a single return value

Returns
-------
float
    Random number generated.

With multiple return values

Returns
-------
length : int
    Length of the returned string.
letters : str
    String of random letters.

jorisvandenbossche pushed a commit to jorisvandenbossche/pandas that referenced this pull request Jun 8, 2018
jorisvandenbossche pushed a commit that referenced this pull request Jun 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype Docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants